Conversation
Contributor
Reviewer's GuideRestructures the project from nested java-api/java-api-stubs packages into a flat src/ and stubs/ layout, updates tooling/config paths accordingly, modernizes tox and mypy wiring, and refreshes documentation to match the new structure and packaging story. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The
.devcontainer.jsonfile is now effectively empty; if devcontainer support is still desired, consider either restoring its previous configuration with updated paths or removing the file entirely to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `.devcontainer.json` file is now effectively empty; if devcontainer support is still desired, consider either restoring its previous configuration with updated paths or removing the file entirely to avoid confusion.
## Individual Comments
### Comment 1
<location> `README.md:39-40` </location>
<code_context>
+### Installing with `pip`
+
+> [!NOTE]
+> For the stubs for this packages, look for [`java-api-stubs`].
+
+The preferred method is to install it by running `pip`. It requires Python
</code_context>
<issue_to_address>
**issue (typo):** Fix grammar in note about stubs package.
Please update the note to use the singular form, e.g. "For the stubs for this package" or "For stub files for this package," to correct the grammar and improve clarity.
```suggestion
> [!NOTE]
> For stub files for this package, look for [`java-api-stubs`].
```
</issue_to_address>
### Comment 2
<location> `README.md:35` </location>
<code_context>
+
+## Installation and usage
+
+To use java-api, you may install it by doing any of the following.
+
+### Installing with `pip`
</code_context>
<issue_to_address>
**suggestion (typo):** Clarify wording about installation methods.
Since only pip installation is documented, consider changing "any of the following" to "the following" or "the following method" to avoid implying multiple options.
```suggestion
To use java-api, you may install it using the following method.
```
</issue_to_address>
### Comment 3
<location> `README.md:49` </location>
<code_context>
+python2 -m pip install java-api
+```
+
+This will install it as package to your Python installation, which will allow
+you to call Ignition Scripting functions from Python's REPL, and get code
+completion using an IDE such as PyCharm and Visual Studio Code.
</code_context>
<issue_to_address>
**issue (typo):** Improve grammar in description of installation result.
Consider rephrasing to: “This will install it as a package in your Python installation,” to correct the article and preposition.
```suggestion
This will install it as a package in your Python installation, which will allow
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| python2 -m pip install java-api | ||
| ``` | ||
|
|
||
| This will install it as package to your Python installation, which will allow |
Contributor
There was a problem hiding this comment.
issue (typo): Improve grammar in description of installation result.
Consider rephrasing to: “This will install it as a package in your Python installation,” to correct the article and preposition.
Suggested change
| This will install it as package to your Python installation, which will allow | |
| This will install it as a package in your Python installation, which will allow |
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Restructure the project layout and tooling around a single top-level java-api package and stubs subpackage, and update documentation to match.
Build:
CI:
Documentation:
Chores: